BUG: can't call `getVal` from `GenExpr` #1148

Zeroto521 · 2026-01-08T12:31:11Z

Introduces the test_evaluate function to verify correct evaluation of various expressions using Model.getVal, including arithmetic and trigonometric operations, and checks for TypeError on invalid input.

Introduced _evaluate methods to Term, Expr, and all GenExpr subclasses for consistent evaluation of expressions and variables. Refactored Solution and Model to use these methods, simplifying and unifying value retrieval for variables and expressions. Cleaned up class definitions and improved hashing and equality for Term and Variable.

Replaced explicit loop with flat iterator for evaluating matrix expressions in Solution.getVal, improving performance and code clarity.

Introduces a new test 'test_evaluate' to verify correct evaluation of matrix variable division and summation in the model.

Eliminated special handling for MatrixExpr and MatrixGenExpr in Solution.__getitem__, simplifying the method to only evaluate single expressions. This change streamlines the code and removes unnecessary numpy array construction.

Renamed 'vars' to 'vartuple' and '_hash' to 'hashval' in the Term class for clarity. Updated all references and methods to use the new attribute names, improving code readability and consistency.

Changed internal evaluation methods in expression classes from float to double for improved numerical precision. Updated type declarations and imports accordingly.

Added type annotations to the Term constructor and specified types for class attributes. This improves code clarity and type safety.

Changed all _evaluate methods in expression classes from cdef to cpdef to make them accessible from Python. Moved the definition of terms and _evaluate to the Expr class in scip.pxd, and refactored Variable to inherit from Expr in scip.pxd, consolidating class member definitions. These changes improve the interface and maintainability of expression evaluation.

Zeroto521 · 2026-01-08T12:33:03Z

src/pyscipopt/expr.pxi

+import math
+from typing import TYPE_CHECKING
+
+from pyscipopt.scip cimport Variable, Solution


This could let IDE know what Variable and Solution are.

But this is only relevant for people working with stuff inside expr.pxi, right?

Zeroto521 · 2026-01-08T12:33:19Z

src/pyscipopt/expr.pxi

 include "matrix.pxi"

+if TYPE_CHECKING:
+    double = float


Same as above

src/pyscipopt/expr.pxi

Ensures that MatrixExpr._evaluate returns a numpy ndarray when appropriate. Adds a test to verify the return type of getVal for matrix variables.

Deleted the _evaluate method from the Solution class in the scip.pyi stub file, as it is no longer needed. Also added TYPE_CHECKING to the typing imports.

Applied the @disjoint_base decorator to the Term and UnaryExpr classes in scip.pyi to clarify their base class relationships. This may improve type checking or class hierarchy handling.

Appended '# noqa: F401' to the 'ClassVar' import to suppress linter warnings about unused imports in scip.pyi.

Zeroto521 · 2026-01-08T13:26:48Z

src/pyscipopt/matrix.pxi

    def __matmul__(self, other):
        return super().__matmul__(other).view(MatrixExpr)

+    def _evaluate(self, Solution sol) -> NDArray[np.float64]:


np.ndarray is not a cdef class, so it can't convert MatrixExpr to a cdef class.
And in MatrixExpr, _evaluate is a pure Python function.

Replaces the @np.vectorize-decorated _evaluate function with an np.frompyfunc-based implementation for evaluating expressions with solutions. This change may improve compatibility and performance when applying _evaluate to arrays.

Refactored the _evaluate method in MatrixExpr to always return the result as a NumPy ndarray, removing the conditional type check.

codecov · 2026-01-09T11:48:43Z

Codecov Report

❌ Patch coverage is 62.79070% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.12%. Comparing base (5ceac82) to head (7cd196a).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
src/pyscipopt/expr.pxi	62.50%	12 Missing ⚠️
src/pyscipopt/matrix.pxi	25.00%	3 Missing ⚠️
src/pyscipopt/scip.pxi	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1148      +/-   ##
==========================================
+ Coverage   55.07%   55.12%   +0.04%     
==========================================
  Files          24       24              
  Lines        5420     5448      +28     
==========================================
+ Hits         2985     3003      +18     
- Misses       2435     2445      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Joao-Dionisio · 2026-01-17T11:16:54Z

tests/test_expr.py

+
+def test_evaluate():
+    m = Model()
+    x = m.addVar(lb=1, ub=1)


Can this please be 2, instead? Just to be safer that it works (a bit paranoid, I know).

Joao-Dionisio · 2026-01-17T11:18:07Z

src/pyscipopt/expr.pxi

+import math
+from typing import TYPE_CHECKING
+
+from pyscipopt.scip cimport Variable, Solution


But this is only relevant for people working with stuff inside expr.pxi, right?

Joao-Dionisio · 2026-01-17T11:18:15Z

src/pyscipopt/expr.pxi

 include "matrix.pxi"

+if TYPE_CHECKING:
+    double = float


Same as above

Joao-Dionisio · 2026-01-17T11:22:03Z

src/pyscipopt/expr.pxi

+    cpdef double _evaluate(self, Solution sol):
+        cdef double res = 1
+        cdef Variable i
+        for i in self.vartuple:


Sorry for my ignorance, but what is a vartuple here? The idea I have in my mind doesn't fit with the result computation below, but my idea is pretty ignorant, admittedly.

Joao-Dionisio · 2026-01-17T11:24:16Z

src/pyscipopt/expr.pxi

+        cdef double coef
+        cdef Term term
+        for term, coef in self.terms.items():
+            res += coef * term._evaluate(sol)


Does this count for when there are higher degrees?

Joao-Dionisio · 2026-01-17T11:24:27Z

src/pyscipopt/expr.pxi

+        cdef GenExpr child
+        cdef double coef
+        for child, coef in zip(self.children, self.coefs):
+            res += coef * child._evaluate(sol)


Same as above.

Joao-Dionisio · 2026-01-17T11:25:47Z

src/pyscipopt/matrix.pxi

    return res.view(MatrixExprCons)


+_evaluate = np.frompyfunc(lambda expr, sol: expr._evaluate(sol), 2, 1)


This shouldn't be here, right?

Joao-Dionisio · 2026-01-17T11:28:41Z

src/pyscipopt/expr.pxi

    def __truediv__(self,other):
        if _is_number(other):
            f = 1.0/float(other)
            return f * self


In the end, this is a much bigger change than I had anticipated. Shouldn't the fix rather be something in here?

Zeroto521 added 10 commits January 7, 2026 22:41

Add tests for expression evaluation in Model

f87432c

Introduces the test_evaluate function to verify correct evaluation of various expressions using Model.getVal, including arithmetic and trigonometric operations, and checks for TypeError on invalid input.

Optimize matrix expression evaluation in Solution

85ee97e

Replaced explicit loop with flat iterator for evaluating matrix expressions in Solution.getVal, improving performance and code clarity.

Add test for matrix variable evaluation

135b954

Introduces a new test 'test_evaluate' to verify correct evaluation of matrix variable division and summation in the model.

Remove matrix expression handling in Solution __getitem__

ad7d4c6

Eliminated special handling for MatrixExpr and MatrixGenExpr in Solution.__getitem__, simplifying the method to only evaluate single expressions. This change streamlines the code and removes unnecessary numpy array construction.

Refactor Term class variable naming and usage

5ff4144

Renamed 'vars' to 'vartuple' and '_hash' to 'hashval' in the Term class for clarity. Updated all references and methods to use the new attribute names, improving code readability and consistency.

Use double precision for expression evaluation

95b3a80

Changed internal evaluation methods in expression classes from float to double for improved numerical precision. Updated type declarations and imports accordingly.

Add type annotations and improve Term class attributes

7770a06

Added type annotations to the Term constructor and specified types for class attributes. This improves code clarity and type safety.

Update CHANGELOG.md

239c3a3

Zeroto521 commented Jan 8, 2026

View reviewed changes

src/pyscipopt/expr.pxi Show resolved Hide resolved

Zeroto521 added 6 commits January 8, 2026 20:46

Remove hash method from Variable

d2ab8e2

back to old behavior

2b145dd

Fix MatrixExpr _evaluate to return ndarray type

9afb07f

Ensures that MatrixExpr._evaluate returns a numpy ndarray when appropriate. Adds a test to verify the return type of getVal for matrix variables.

Remove unused _evaluate method from Solution stub

66c2f6b

Deleted the _evaluate method from the Solution class in the scip.pyi stub file, as it is no longer needed. Also added TYPE_CHECKING to the typing imports.

Add @disjoint_base decorator to Term and UnaryExpr

4ff2682

Applied the @disjoint_base decorator to the Term and UnaryExpr classes in scip.pyi to clarify their base class relationships. This may improve type checking or class hierarchy handling.

Add noqa to suppress unused import warning

073ac1c

Appended '# noqa: F401' to the 'ClassVar' import to suppress linter warnings about unused imports in scip.pyi.

Zeroto521 commented Jan 8, 2026

View reviewed changes

Zeroto521 added 3 commits January 9, 2026 17:32

cache _evaluate function for matrix

22b9f4f

Refactor _evaluate to use np.frompyfunc

449e2fd

Replaces the @np.vectorize-decorated _evaluate function with an np.frompyfunc-based implementation for evaluating expressions with solutions. This change may improve compatibility and performance when applying _evaluate to arrays.

Simplify _evaluate return in MatrixExpr

7cd196a

Refactored the _evaluate method in MatrixExpr to always return the result as a NumPy ndarray, removing the conditional type check.

Merge branch 'master' into issue/1062

3ff4658

Joao-Dionisio reviewed Jan 17, 2026

View reviewed changes

		return res.view(MatrixExprCons)


		_evaluate = np.frompyfunc(lambda expr, sol: expr._evaluate(sol), 2, 1)

BUG: can't call getVal from GenExpr #1148

Are you sure you want to change the base?

BUG: can't call getVal from GenExpr #1148

Conversation

Zeroto521 commented Jan 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Zeroto521 Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 9, 2026

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

BUG: can't call `getVal` from `GenExpr` #1148

BUG: can't call `getVal` from `GenExpr` #1148

Zeroto521 Jan 8, 2026 •

edited

Loading